Updated color-me module by hfgd123 · Pull Request #206 · ScootKit/CustomDCBot

hfgd123 · 2026-07-28T15:24:04Z

to support color gradients and holographic role names

Suggestion: https://featureboard.net/suggestions/e92054de-c920-449b-a27d-2e6b97746f41

…le names Suggestion: https://featureboard.net/suggestions/e92054de-c920-449b-a27d-2e6b97746f41

hfgd123 · 2026-07-28T16:56:31Z

@copilot review

Copilot

Copilot was unable to review this pull request because the user who requested the review has reached their quota limit.

Copilot

Pull request overview

Copilot reviewed 8 out of 8 changed files in this pull request and generated 2 comments.

Comments suppressed due to low confidence (7)

modules/color-me/commands/color-me.js:29

The new allowEnhancedRoleColors config option is not applied here: multiColor only checks the guild feature, so disabling the setting has no effect. Consider factoring the config into the multiColor flag (and compute it after loading moduleConf).

        const multiColor = client.guild.features.includes('ENHANCED_ROLE_COLORS');
        if (!multiColor && (interaction.options.getString('secondary-color') !== null || interaction.options.getBoolean('holographic'))) {
            colorfulW = false;
        }

modules/color-me/commands/color-me.js:74

When editing an existing role in-place, the DB record (including timestamp) is not updated. This breaks the cooldown behavior (users can keep editing after the initial cooldown window) and leaves stored colors/name stale.

                await role.edit(
                    {
                        name: interaction.options.getString('name'),
                        colors: isHolographic ? Constants.HolographicStyle : {
                            primaryColor: primaryColor,

modules/color-me/commands/color-me.js:122

roles.cache.has(...) expects a role ID (string). At this point role is a Role object returned from guild.roles.create(), so this check will always be false and the member role will always be re-added unnecessarily.

                if (!interaction.member.roles.cache.has(role)) {

modules/color-me/configs/strings.json:36

This rename introduces updatedLimited, but the configuration localization metadata still defines updatedNoIcon (see config-localizations/en.json). If config generation/verification depends on that metadata, this will cause missing/unused string keys.

      "name": "updatedLimited",
      "humanName": "Role updated with limited features",
      "default": "Your role was updated successfully, but either your role icon or enhanced role colors were not used, as these features need to be unlocked by boosting the server.",
      "description": "This messages gets send when a booster sucessfully updates their custom role, but the guild has not enough boosts to use either role icons or enhanced role colors",
      "type": "string",
      "allowEmbed": true

modules/color-me/configs/strings.json:34

Grammar/spelling in this newly added/modified description is incorrect ("messages gets send", "sucessfully").

      "description": "This messages gets send when a booster sucessfully updates their custom role, but the guild has not enough boosts to use either role icons or enhanced role colors",

modules/color-me/configs/config.json:46

allowEnhancedRoleColors is added to the module config, but it is missing from config-localizations/en.json, so the config UI/metadata may not expose a humanName/description for it (and config verification may fail if it expects a localization entry per config key).

      "name": "allowEnhancedRoleColors",
      "humanName": "Allow \"Enhanced Role Colors\"",
      "default": true,
      "description": "Should the module allow users to use the \"Enhanced Role Colors\" feature? (If your server doesn't have this feature unlocked, this setting will have no effect)",
      "type": "boolean"

modules/color-me/commands/color-me.js:66

New behavior paths (secondary color, holographic mode, and the updatedLimited reply when features are unavailable) are not covered by unit tests in tests/color-me/manage.test.js. Adding tests for these branches would help prevent regressions, especially around feature-flag/config interactions.

        const isHolographic = interaction.options.getBoolean('holographic') && multiColor;
        if (role) {

SCDerox

Thanks for taking this on - enhanced role colors are a genuinely nice addition, and the migration file, the config-localization-aware structure and the test updates show you looked at how the rest of the codebase does things.

About this review: it was put together with AI assistance and then checked over by me. The branch was pulled locally and the findings below were reproduced against real discord.js rather than the test mocks. If any point here looks wrong or you disagree with it, just reply to that comment - I'll go through it manually and we'll sort it out.

The unit tests pass, but they mock roles.create / role.edit as jest.fn(), so nothing in the suite exercises how discord.js actually interprets the payload - and that turns out to be where the main problem is. Details are in the inline comments; the summary:

Blocking

Holographic roles throw on every request. Constants.HolographicStyle is the wrong shape for colors, and discord.js throws before the request is sent. See the comment on color-me.js line 74.
allowEnhancedRoleColors has no functional effect. It only picks the reply message; enhanced colors still get applied when it's off.
The "limited" message fires when nothing was limited. Turning the setting off makes every manage call report a degraded result.

Should be fixed before merge

The inverted secondary-color fallback in guildMemberUpdate.js, and the edit path never writing the new columns back to the DB - both inline below.

One question: was the holographic path tested against a live guild? Given finding 1 it should fail every time, so I'd like to know if there's a code path being misread here.

I'll handle the scnx-docs update on my side once this lands - the docs page is stale on the renamed option, the two new options, the new config field and the stored-data list.

Co-authored-by: Simon <simon@scootkit.com>

SCDerox

Second pass. Everything from the last round is correctly addressed - the HolographicStyle shape, folding allowEnhancedRoleColors into multiColor, the secondary-colour validation ordering, the '0x000000' -> null fallback, defaultValue: false, the typo, and the test assertion. Thanks for working through all of it.

Two new blockers came in with those fixes, though, and both are in the persistence layer:

Colours are stored as raw integers and can't be read back - resolveColor() throws on the reboost recreate for every role created or edited after this PR (verified end-to-end).
role.edit() returns a clone, so the DB write-back added in a199239 persists the pre-edit name and colours - it doesn't actually fix the staleness it was meant to fix.

Beyond those: the event path still ignores allowEnhancedRoleColors, createdNoIcon didn't get the updatedLimited treatment, and config-localizations/en.json needs regenerating.

Verified as fine, for what it's worth: ENHANCED_ROLE_COLORS is the right feature string, the migration filename and tables export match the runner's contract, and the migration is idempotent and preserves existing data (I ran it against a pre-PR table). Nice to see the first module migration in this repo land cleanly.

SCDerox · 2026-08-08T19:17:29Z

+                    primaryColor: role.colors.primaryColor,
+                    secondaryColor: role.colors.secondaryColor,
+                    holo: !!role.colors.tertiaryColor,


Blocking - these write raw integers into a STRING column, and the recreate path can't read them back.

role.colors.primaryColor is data.colors.primary_color, a raw integer (discord.js/src/structures/Role.js:82) - not a hex string. The old code stored role.hexColor. The column has TEXT affinity, so SQLite coerces on write and hands back a decimal string:

read back: { primaryColor: '11127295', secondaryColor: '16759788', holo: 1 } resolveColor('11127295') -> DiscordjsTypeError [ColorConvert]: Unable to convert "11127295" to a number.

guildMemberUpdate.js:49-50 feeds that value straight into roles.create({colors}), which calls resolveColor() on it. So with recreateRole enabled, any role created or edited after this PR throws on the unboost -> reboost recreate.

It also splits the column into two formats: rows migrated from the old color column keep hex (I ran the migration - #f1c40f survives intact), new rows get decimals.

Storing hex keeps the column single-format and needs no value migration. There's only a hexColor getter for the primary, so it needs a small helper - which is why this isn't a one-click suggestion:

const toHex = (c) => (c === null || c === undefined ? null : '#' + c.toString(16).padStart(6, '0'));

then primaryColor: toHex(role.colors.primaryColor), secondaryColor: toHex(role.colors.secondaryColor). Same three lines repeat at 140-142 and 185-187.

SCDerox · 2026-08-08T19:17:29Z

            if (interaction.guild.roles.cache.find(r => r.id === role)) {
                role = interaction.guild.roles.resolve(role);
-                role.edit(
+                await role.edit(


Blocking - role is never patched by edit(), so the write-back below saves stale values.

RoleManager#edit returns a clone and leaves the original untouched (discord.js/src/managers/RoleManager.js:295-297):

const clone = role._clone(); clone._patch(d); return clone;

and _clone() is Object.assign(Object.create(this), this), so _patch writes a fresh colors object onto the clone only.

That means at line 96-99 role.name and role.colors are still the pre-edit values, and the row records what the role looked like before the edit - which is exactly the staleness the write-back was added to fix. (The cached role does get patched eventually by the GUILD_ROLE_UPDATE gateway event, but that's a race, not a guarantee, and it definitely hasn't landed by the next line.)

Assigning the return value fixes it:

Suggested change

await role.edit(

role = await role.edit(

SCDerox · 2026-08-08T19:17:29Z

+                    primaryColor: role.colors.primaryColor,
+                    secondaryColor: role.colors.secondaryColor,
+                    holo: !!role.colors.tertiaryColor,


Same integer-vs-hex problem as line 97 - this is the create-after-record-exists path.

SCDerox · 2026-08-08T19:17:29Z

+                    primaryColor: role.colors.primaryColor,
+                    secondaryColor: role.colors.secondaryColor,
+                    holo: !!role.colors.tertiaryColor,


Same integer-vs-hex problem again - the first-time create path.

SCDerox · 2026-08-08T19:17:29Z

-                const color = data.color;
+                const primaryColor = data.primaryColor;
+                const secondaryColor = data.secondaryColor;
+                const isHolographic = data.holo && client.guild.features.includes('ENHANCED_ROLE_COLORS');


allowEnhancedRoleColors got folded into multiColor in color-me.js, but the event path still checks only the guild feature. So an admin who turns the setting off still gets holographic roles recreated on reboost - the switch is still half-inert.

moduleConf is already in scope from line 10, and hoisting the check into a local also cleans up the duplicate features.includes() on line 62:

Suggested change

const isHolographic = data.holo && client.guild.features.includes('ENHANCED_ROLE_COLORS');

const enhancedColors = client.guild.features.includes('ENHANCED_ROLE_COLORS') && moduleConf['allowEnhancedRoleColors'];

const isHolographic = data.holo && enhancedColors;

SCDerox · 2026-08-08T19:17:29Z


 const strings = {invalidColor: 'invalid'};

 test('returns default gold colour and no cancel when no colour is given', async () => {


Test name no longer matches what it asserts - the default is 0x000000 now, not gold.

Suggested change

test('returns default gold colour and no cancel when no colour is given', async () => {

test('returns the default colour and no cancel when no colour is given', async () => {

SCDerox · 2026-08-08T19:17:29Z

-      "humanName": "Role updated without icon",
-      "default": "Your role was updated successfully, but your role icon was not used, as this requires the guild to be boost level 2 or higher.",
-      "description": "This messages gets send when a booster sucessfully updates their custom role, but the guild has not enough boosts to use role icons",
+      "name": "updatedLimited",


Heads-up rather than a change request: renaming the key means any server that customised updatedNoIcon silently loses that text and reverts to this new default (configuration.js falls back to the field default when a stored key is absent). Fine given the meaning genuinely changed - just a one-way reset worth knowing about.

SCDerox · 2026-08-08T19:17:29Z

+    expect(i.guild.roles.create).toHaveBeenCalledWith(expect.objectContaining({
+        colors: expect.objectContaining({secondaryColor: null})
+    }));
+    expect(i.editReply).toHaveBeenCalledWith(expect.objectContaining({}));


expect.objectContaining({}) matches any object, so this assertion can never fail - it should either assert the actual string (updated-limited) or come out.

More broadly: nothing here covers the edit path's new DB write-back, and nothing covers the holographic recreate in guildMemberUpdate. Those are the two paths carrying the blocking bugs on this review, which is how they got past green CI. A test that asserts the persisted primaryColor round-trips back through resolveColor() would catch the integer/hex issue directly.

SCDerox · 2026-08-08T19:17:29Z

 const {localize} = require('../../../src/functions/localize');
 const {client} = require('../../../main');
 const {embedType, dateToDiscordTimestamp} = require('../../../src/functions/helpers');
+const { Constants } = require('discord.js');


Nit: const { Constants } here vs const {Constants} in guildMemberUpdate.js:2 and the rest of the codebase. @stylistic/object-curly-spacing is off so lint won't catch it.

SCDerox · 2026-08-08T19:17:29Z

+            if (description.holo) await queryInterface.removeColumn(TABLE, 'holo', {transaction});
+        });
+    }
+};


Nit: no trailing newline. (@stylistic/eol-last is off, so this is cosmetic.)

Updated color-me module to support color gradients and holographic ro…

416e14e

…le names Suggestion: https://featureboard.net/suggestions/e92054de-c920-449b-a27d-2e6b97746f41

hfgd123 changed the base branch from main to closed-source-sync-july July 28, 2026 16:58

SCDerox requested a review from Copilot July 28, 2026 17:03

Copilot AI reviewed Jul 28, 2026

hfgd123 requested a review from Copilot July 28, 2026 17:04

Copilot started reviewing on behalf of hfgd123 July 28, 2026 17:05 View session

Copilot AI reviewed Jul 28, 2026

View reviewed changes

Comment thread modules/color-me/commands/color-me.js Outdated

Comment thread modules/color-me/models/Role.js Outdated

hfgd123 added 4 commits July 28, 2026 19:11

Fixed not checking for disabled role colors

aed8314

Fixed guildMemberUpdate.js

71d25b4

Fixed guildMemberUpdate.js

457c1b0

Fixed tests

e0324c0

SCDerox requested changes Jul 31, 2026

View reviewed changes

hfgd123 and others added 9 commits July 31, 2026 19:52

Update modules/color-me/commands/color-me.js

ab0fe07

Co-authored-by: Simon <simon@scootkit.com>

Fixed color constant

d731fef

MultiColor Check secondary thing

53bda0e

Updating DB when role is edited

a199239

Update modules/color-me/events/guildMemberUpdate.js

7c4a2de

Co-authored-by: Simon <simon@scootkit.com>

Update modules/color-me/configs/strings.json

7d169c5

Co-authored-by: Simon <simon@scootkit.com>

Update modules/color-me/migrations/colorme_Role__V1.js

e08d187

Co-authored-by: Simon <simon@scootkit.com>

Changed DB Model

9772e66

Update tests/color-me/manage.test.js

7eb74cb

Co-authored-by: Simon <simon@scootkit.com>

hfgd123 requested a review from SCDerox July 31, 2026 18:22

SCDerox requested changes Aug 8, 2026

View reviewed changes

	const isHolographic = data.holo && client.guild.features.includes('ENHANCED_ROLE_COLORS');
	const enhancedColors = client.guild.features.includes('ENHANCED_ROLE_COLORS') && moduleConf['allowEnhancedRoleColors'];
	const isHolographic = data.holo && enhancedColors;


		const strings = {invalidColor: 'invalid'};

		test('returns default gold colour and no cancel when no colour is given', async () => {

	test('returns default gold colour and no cancel when no colour is given', async () => {
	test('returns the default colour and no cancel when no colour is given', async () => {

Uh oh!

Conversation

hfgd123 commented Jul 28, 2026

Uh oh!

hfgd123 commented Jul 28, 2026

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Pull request overview

Uh oh!

Uh oh!

Uh oh!

SCDerox left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

SCDerox left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants